-
Notifications
You must be signed in to change notification settings - Fork 216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Base Names testnet compatibility #966
Conversation
Deployment failed with the following error:
View Documentation: https://vercel.com/docs/accounts/team-members-and-roles |
41a85fb
to
90cac3d
Compare
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -61,6 +61,7 @@ export type UseGetTokenBalanceResponse = { | |||
*/ | |||
export type WalletContextType = { | |||
address?: Address | null; // The Ethereum address to fetch the avatar and name for. | |||
chain?: Chain; // Optional chain for domain resolution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I think we probably want to start move away from using chain
as type. Mostly because we are seeing occasional missmatch with Viem types.
Let's do instead chainId
for those things going forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thoughts on merging this and then updating to chainId in the next PR? Because the chainId change will require getName
, Name
, and useName
updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll push up the chainId changes and we can split it into two PRs if it's too large
a812b14
to
9538ec4
Compare
f31f486
to
b0d6c89
Compare
b0d6c89
to
ab9ed21
Compare
ab9ed21
to
e36b444
Compare
e36b444
to
c3d4dab
Compare
c3d4dab
to
81720f1
Compare
81720f1
to
7d0c862
Compare
7d0c862
to
658d0db
Compare
658d0db
to
b837f57
Compare
b837f57
to
821fd8c
Compare
821fd8c
to
9a16b9a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing this.
Make sure to follow up tomorrow with a updated version of WalletContextType
type.
We should never ship that changes a type without the docs updated as well.
What changed? Why?
Base Names was only working on Mainnet. This updates the component to work with EOAs and Smart Wallets on Mainnet and Testnet.
Notes to reviewers
How has it been tested?